Share one adapter for putting ezmsg streams on phosphor plots - #9
Merged
Conversation
Three places were doing this independently -- sigmon, viewer, and a downstream shmem viewer -- and each had a different subset of it right. The one that matters: both CLIs computed channel count as `data.size // n_samples` and reshaped to match, which turns a (time, ch, 2) min/max envelope into twice as many traces alternating lower and upper bounds, against labels now off by a factor of two. It renders, so nothing complains. ezmsg.tools.plot.describe is the pure half: given dims, axes and attrs, work out channel count, rate, labels, unit, and whether this is an envelope. It imports neither Qt nor phosphor, so it serves a topic subscriber, a shmem mirror, and a test with neither, and it is where the reshaping rule lives. An envelope is identified by its axis *labels*, not its name or width. A 2-wide trailing axis could as easily be (mean, std), which would be nonsense drawn as bounds, so the coordinates have to say min and max. The name only narrows the search, since ezmsg-sigproc's BinnedAggregate lets callers rename it. ezmsg.tools.plot.ShmemSweepWidget is the Qt half: mirror a ring, build the plot on first data, poll at the render cadence, and reconfigure in place when the stream changes shape rather than rebuilding -- a rebuild flashes the plot every time a user narrows their channel selection. It rebuilds only when the rate or envelope mode changes, since those invalidate the buffer's whole layout. The rate it hands the buffer is the ring header's, which for an envelope is already the bucket rate. That is the one that is easy to get wrong: sizing from the pre-decimation rate makes the ring `factor` times longer than the data arriving to fill it, and the sweep sits mostly empty. sigmon and viewer both move onto describe, which is what fixes the envelope handling in each and is the evidence the abstraction is not just one consumer's widget with the serial numbers filed off. 13 tests, none needing Qt or a GPU.
A trailing 2-wide axis was being treated as a min/max envelope, which is right for BinnedAggregate(MIN, MAX) and wrong for anything else with the same shape. (mean, std) is the obvious other case: same rank, same width, completely different drawing -- a semi-transparent band from mean-std to mean+std with an opaque line at the mean, and a column reduction that averages rather than takes extremes. So the axis is now identified by a vocabulary. METRIC_KINDS maps label tuples to a kind; SWEEP_RENDERABLE_METRICS says which of those a sweep can actually draw, which today is minmax alone. Adding a kind to the first is cheap; adding it to the second means teaching a renderer. Describing and drawing are separated deliberately. describe_* reports any recognised metric, including ones nothing here can draw, because a caller asking what arrived should not have to catch an exception to find out. require_sweep_renderable is the capability check, and it names the kind, the labels and what is supported rather than just refusing. The widget reports and stops rather than raising out of its Qt timer slot, which would repeat the failure every tick for as long as the window stayed open. The message goes where the plot would have been. Labels are matched case-insensitively, and an unrecognised vocabulary is not an error -- it falls through to being ordinary extra dimensions, which is the pre-existing behaviour for any trailing axis.
`phosphor = { path = "../phosphor", editable = true }` is a developer
convenience for testing against an unreleased phosphor. It went in with
the adapter commit by accident, and CI has no sibling checkout:
error: Distribution not found at: file:///home/runner/work/ezmsg-tools/phosphor
A bad path source fails resolution for *every* job, including ones that
never touch the package -- the failure above is `uv sync --only-group
docs`, which needs neither phosphor nor Qt.
Left a note next to the existing commented-out ezmsg git source saying
how to add one locally and that it should not be committed.
on_plot_built and on_frame are for the parts a host application has that this widget cannot: overlays and diagnostics parented to a plot that does not exist until the stream does, and readouts needing units the widget has no way to interpret. Without them a subclass has to override _on_tick and _build, which are internals. on_frame fires whether or not samples arrived, so a subclass sees a steady cadence rather than one that stalls whenever the source goes quiet. _needs_rebuild pulls the rebuild-versus-resize decision out of _apply_shape so it can be tested without a ring, a GPU or a display. It is worth pinning: rebuilding throws away the figure and flashes the plot, which happens every time a user narrows their channel selection if the rule is too eager, and a rate or metric change that is *not* caught leaves the buffer laid out for the wrong data.
ShMemCircBuff wrote two descriptions of the same buffer and they disagreed. on_message rolls the buffered axis to the front, and meta.shape was already written in that order -- but the metadata blob recorded msg.dims, the order the message arrived in. For the common (time, ch) case those coincide, so it went unnoticed; for a source sending (ch, time) the header said one thing and dims said another. describe_mirror was compensating by re-rolling dims itself, which is the wrong place: a reader has no way to know it should, and any other consumer of mirror.dims would have got it wrong. The sink knows what it stored, so it says so. Fixed at the source and the compensation removed. A test pins that a transposed source is described by the buffer's order.
Rebuilding logs a rendercanvas "Present finish error: Internal C++ object already deleted". The plot works either way, but the sequence is genuinely loose: figure.close() is followed by deleteLater, which defers the C++ destruction to the next event-loop turn, leaving a window in which rendercanvas can try to present a frame into a widget on its way out. Hiding first narrows it -- a hidden widget receives no paint events -- and is correct ordering regardless of whether it silences the message. Unverified: reproducing needs a live canvas, and there is no GUI harness here. If the message persists, the next thing to try is stopping the canvas scheduler explicitly before close rather than relying on close to do it.
A channel-selection change logs two "Created shmem at sm_..." lines back
to back, and there was no way to tell from them what the two were: the
names are sha256-hashed to fit the platform's length limit, so both look
like noise. They are the data ring, reallocated because the frame shape
changed, and the metadata blob, republished because the ch axis lost
channels -- both necessary, neither obvious.
Created data ring gen 1 (1000x64) (256000 bytes) at sm_PkyA3Ki...
Created stream metadata gen 2 (4448 bytes) at sm_hzb4AM68...
The generation and shape are the useful part: they say which reset this
was and how big the result is, which is what someone chasing a blanked
plot or a memory question actually wants.
Also fixes the retry counter, which was never incremented, so every line
claimed "in 0 attempts" whether or not a stale name had been cleaned up
first -- a number that could only ever read zero. Now the retries are
mentioned only when there were some, which is the case worth noticing.
phosphor is what viewer and sigmon plot with; nothing else here needs it.
In core it would put fastplotlib, PySide6 and a GPU stack behind every
install -- so ezmsg-performance-monitor, a Dash web app, would ship Qt --
and raise the core numpy floor from 1.26 to 2.4.2. It stays in the two
extras, now at >=0.7.0 for the envelope input, set_channel_labels,
set_channel_labels_visible and SweepConfig.max_fps.
What made core look necessary is real, but it is not about imports:
console scripts are installed unconditionally, because [project.scripts]
has no notion of extras. `pip install ezmsg-tools` puts
ezmsg-signal-monitor on the PATH and running it gives a bare
ModuleNotFoundError with no hint that an extra exists or what it is
called. perfmon has the identical problem with dash.
Both entry points now go through a guard that names the extra:
ezmsg-signal-monitor needs the optional 'sigmon' dependencies
(could not import 'ezmsg.qt').
Install them with:
pip install 'ezmsg-tools[sigmon]'
It reports the module that was actually missing rather than assuming the
extra is the whole story -- an ImportError from inside the CLI is a
different problem, and saying which one it was keeps the message honest.
Making phosphor properly optional then exposed a bug in ezmsg.tools.plot.
Its __init__ eagerly imported the Qt widget, so importing the deliberately
Qt-free .describe submodule pulled in a GPU stack anyway -- a submodule
import runs its parent's __init__ first. It never showed while phosphor
was always installed. ShmemSweepWidget is now resolved lazily via PEP 562
__getattr__, so the public name still works where phosphor is present and
.describe is importable where it is not. Verified on a clean install with
neither phosphor nor Qt present.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three places were doing this independently —
sigmon,viewer, and a downstream shmem viewer — and each had a different subset of it right.The bug this fixes
Both CLIs computed channel count as
data.size // n_samplesand reshaped to match:Given a
(time, ch, 2)min/max envelope — whatezmsg-sigproc'sBinnedAggregateproduces with a tupleoperation, and what the shmem bridge now carries — that yieldsn_channels = ch * 2and renders twice as many traces, alternating lower and upper bounds, against channel labels now off by a factor of two. It renders, so nothing complains.What's here
ezmsg.tools.plot.describe— the pure half. Given dims, axes and attrs, work out channel count, sample rate, labels, unit, and whether this is an envelope. Imports neither Qt nor phosphor, so it serves a topic subscriber, a shared-memory mirror, and a test with neither, and it is the single place the reshaping rule lives.An envelope is identified by its axis labels, not its name or width. A 2-wide trailing axis could as easily be
(mean, std), which would be nonsense drawn as bounds, so the coordinates have to sayminandmax. The name only narrows the search, sinceBinnedAggregatelets callers rename it.ezmsg.tools.plot.ShmemSweepWidget— the Qt half. Mirror a ring, build the plot on first data (channel count, rate and names are properties of the stream, which may not exist when the window opens), poll at the render cadence, and reconfigure in place when the stream changes shape. A rebuild flashes the plot, which is unpleasant every time a user narrows their channel selection; it rebuilds only when the rate or envelope mode changes, since those invalidate the buffer's whole layout.The rate it hands the buffer is the ring header's, which for an envelope is already the bucket rate. That is the trap: sizing from the pre-decimation rate makes the ring
factortimes longer than the data arriving to fill it, and the sweep sits mostly empty.Both CLIs move onto
describe, which fixes the envelope handling in each and is the evidence the abstraction isn't just one consumer's widget with the serial numbers filed off.Tests
13 new, none needing Qt or a GPU —
describeis deliberately pure. 33 pass across the plot and shmem suites.Requires
phosphor's envelope support,
set_channel_labels,set_channel_labels_visibleandSweepConfig.max_fps— ezmsg-org/phosphor#4. Thephosphor>=0.5.0floor in thesigmon/viewerextras must be bumped to whatever release carries those before this is published; there is a comment inpyproject.tomlsaying so.Not verified
ShmemSweepWidgethas no test coverage and has not been run — it needs a live ring, a GPU and a display. Its pure inputs (describe,flatten_for_plot) are covered; the widget wiring around them is not.sigmon/cli.pyis compile-checked but not import-checked here:pygraphvizwill not build in this environment (no graphviz headers).viewer/cli.pyimports cleanly under theviewerextra.